Skip to content

Proposed fix for zoom out issue #384#596

Open
mitchjb-sos wants to merge 1 commit intoembedpdf:mainfrom
mitchjb-sos:Fix/zoom-out-always-10%
Open

Proposed fix for zoom out issue #384#596
mitchjb-sos wants to merge 1 commit intoembedpdf:mainfrom
mitchjb-sos:Fix/zoom-out-always-10%

Conversation

@mitchjb-sos
Copy link
Copy Markdown

This is a proposed fix for #384.

I am not sure about the original intention of the math or the feature, but the issue arises due to differences between how browsers report deltaY with mouse wheel scrolling.

I believe the values that are reported for various browser:
Chromium: +/-100
Firefox: +/-3
Safari: Hardware accelerated, aka, values are provided by mouse

When users with Chromium browsers attempt to Ctrl + scroll, accumulatedWheelScale ends up being clamped to 2/0.1, hence the reported zoom out by 10%:

const handleWheel = (e: WheelEvent) => {
    if (!e.ctrlKey && !e.metaKey) return;
    e.preventDefault();

    if (wheelZoomTimeout === null) {
      initialZoom = getState().currentZoomLevel;
      accumulatedWheelScale = 1;
      initializeGestureState(e.clientX, e.clientY);
    } else {
      clearTimeout(wheelZoomTimeout);
    }
   
    // the deltaY always = 100 then zoomFactor always =0, so the accumulatedWheelScale always = 0.1
    // the deltaY always = 100 then zoomFactor always =0, so the accumulatedWheelScale always = 0.1
    // the deltaY always = 100 then zoomFactor always =0, so the accumulatedWheelScale always = 0.1

    const zoomFactor = 1 - e.deltaY * 0.01;
    accumulatedWheelScale *= zoomFactor;
    accumulatedWheelScale = Math.max(0.1, Math.min(10, accumulatedWheelScale));
    updateTransform(accumulatedWheelScale);

    wheelZoomTimeout = setTimeout(() => {
      wheelZoomTimeout = null;
      commitZoom();
      accumulatedWheelScale = 1;
    }, 150);
  };

(Special thanks to lystormenvoy who helped point to the source code where the issue resides.)


The proposed solution is as follows:

// Utilizing deltaY sign instead of raw value to eliminate discrepancies between browsers
const zoomFactor = 1 - Math.sign(e.deltaY) * zoomStep; // Should this use zoomStep configured by the plugin config?
accumulatedWheelScale *= zoomFactor;
accumulatedWheelScale = clamp(accumulatedWheelScale, 0.1, 10);

I changed the zoomFactor calculation to utilize only the sign, or direction, from the scroll and apply a static zoomStep (which perhaps should be utilizing the zoomStep config from the zoom plugin config, as you can see in my comment).

I also changed to the clamp function that I saw defined above for limiting accumulatedWheelScale.

I attempted to modify all corresponding props and APIs for the various frameworks, but I am really only a React dev and it would be good to review all of the code before merging!

Lastly, this is my first ever FOSS pull request, so please feel free to give gentle feedback! 😃

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 14, 2026

@mitchjb-sos is attempting to deploy a commit to the OpenBook Team on Vercel.

A member of the Team first needs to authorize it.

@mitchjb-sos mitchjb-sos changed the title Proposed fix for zoom out issue Proposed fix for zoom out issue #384 Apr 14, 2026
@lystormenvoy
Copy link
Copy Markdown

well done

@mitchjb-sos mitchjb-sos mentioned this pull request Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants